Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix: CSS to animate popups, dialog for accessibility (fixes #600) #601

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

Fix: CSS to animate popups, dialog for accessibility (fixes #600) #601

wants to merge 18 commits into from

Conversation

oliverfoster
Copy link
Member

@oliverfoster oliverfoster commented Oct 29, 2024 •
edited
Loading

fixes #600
requires adaptlearning/adapt-contrib-vanilla#531

Changed notify and drawer to dialog elements from div elements. Formalised shadow as an API rather than as part of drawer. Switched to CSS animations for drawer, notify and shadow.

dialog element improves popup accessibility by using the browser to limit tabbing and readability rather than Adapt's algorithm.

CSS animations provide a much smoother viewing experience as shows and slides for notify and drawer are rendered perfectly and without their usual jankiness. The show and hide behaviour of these modules will be able to be changed more easily.

Update

  • Changed notify to dialog element
  • Changed drawer to dialog element

New

  • added transitions.js:transitionNextFrame function to wait for one requestAnimationFrame allowing css changes to settle
  • a11y/popup.js allow popup api to handle dialog elements, added deprecation warning to non dialog popup elements
  • formalised the shadow api with isOpen, show, hide
  • added shadow, notify and drawer animations on classes anim-show-before, anim-show-after, anim-hide-before, anim-hide-after
  • added cubic-bezier easing css variables --adapt-cubic-bezier-linear, --adapt-cubic-bezier-ease, --adapt-cubic-bezier-easein, --adapt-cubic-bezier-easeout, --adapt-cubic-bezier-eastinout, --adapt-cubic-bezier-easeinsine, --adapt-cubic-bezier-easeoutsine, --adapt-cubic-bezier-easeinoutsine, --adapt-cubic-bezier-easeinquad, --adapt-cubic-bezier-easeoutquad, --adapt-cubic-bezier-easeinoutquad, --adapt-cubic-bezier-easeincubic, --adapt-cubic-bezier-easeoutcubic, --adapt-cubic-bezier-easeinoutcubic, --adapt-cubic-bezier-easeinquart, --adapt-cubic-bezier-easeoutquart, --adapt-cubic-bezier-easeinoutquart, --adapt-cubic-bezier-easeinquint, --adapt-cubic-bezier-easeoutquint, --adapt-cubic-bezier-easeinoutquint, --adapt-cubic-bezier-easeinexpo, --adapt-cubic-bezier-easeoutexpo, --adapt-cubic-bezier-easeinoutexpo, --adapt-cubic-bezier-easeincirc, --adapt-cubic-bezier-easeoutcirc, --adapt-cubic-bezier-easeinoutcirc, --adapt-cubic-bezier-easeinback, --adapt-cubic-bezier-easeoutback, --adapt-cubic-bezier-easeinoutback
  • drawer and notify use css animation, added css variables to control animations --adapt-drawer-duration, --adapt-drawer-show-easing, --adapt-drawer-hide-easing, --adapt-notify-duration
  • added schema config.json:notify._duration

Breaking

  • Removed drawer elastic and bounce transition easing options as they cannot be creating in css, forcing qunit and back transitions instead

References

@oliverfoster oliverfoster added the enhancement New feature or request label Oct 29, 2024
@oliverfoster oliverfoster self-assigned this Oct 29, 2024
@oliverfoster oliverfoster mentioned this pull request Oct 29, 2024
Open
kirsty-hames
kirsty-hames reviewed Oct 30, 2024
View reviewed changes
Copy link
Contributor

@kirsty-hames kirsty-hames left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is working great thanks @oliverfoster. I've tested the tabbing and screen reader capability in comparison to the current functionality (master). Drawer dialog seems consistent, but I've noted a couple of differences for Notify dialog. Not sure if these are expected or need further input?

  • Notify dialog popup title doesn't display visual focus anymore for Chrome or Safari (Mac). FF (Mac + Win) is intermittent. Chrome and Edge (Win) work as expected.

  • For Edge (Win) and FF (Mac), when tabbing through Notify dialog popup, after the close btn, focus is directed to the browser UI. Focus does loop and return to the popup elements e.g. title > prev btn > next btn > close btn > browser UI > title > prev btn etc. For Chrome (Mac + Win) and Safari (Mac), focus is contained to the popup only.

swashbuck
swashbuck reviewed Oct 30, 2024
View reviewed changes
js/views/drawerView.js Outdated Show resolved Hide resolved
@oliverfoster
Copy link
Member Author

oliverfoster commented Oct 31, 2024 •
edited
Loading

Not sure if these are expected or need further input?

  • Notify dialog popup title doesn't display visual focus anymore for Chrome or Safari (Mac). FF (Mac + Win) is intermittent. Chrome and Edge (Win) work as expected.

I have removed the a11y.focusNext call to allow the browser to handle it through the native dialog.showModal function. Adapt therefore no longer tries to focus on the title.

  • For Edge (Win) and FF (Mac), when tabbing through Notify dialog popup, after the close btn, focus is directed to the browser UI. Focus does loop and return to the popup elements e.g. title > prev btn > next btn > close btn > browser UI > title > prev btn etc. For Chrome (Mac + Win) and Safari (Mac), focus is contained to the popup only.

I will make sure that the default browser behaviour is utilised for the loop around. We should not stop the focus from looping around into the browser UI if that is expect.

@swashbuck
Copy link
Contributor

swashbuck commented Oct 31, 2024 •
edited
Loading

I've removed the Drawer easing properties from the schemas in 4e00cad. These reference easing strings that are specific to Velocity JS and are not available as a CSS transition-timing-function value.

These easing methods could be reproduced using cubic-bezier() values, but we would want to set up new variables in the theme (ex. @animation-easing-easeInOutQuart). Currently, I've just switched all the easing functions to use @animation-easing in the Vanilla PR. Not sure that it's worth the effort to support Drawer easing options via config?

@oliverfoster
Copy link
Member Author

oliverfoster commented Nov 4, 2024 •
edited
Loading

I've removed the Drawer easing properties from the schemas [..]
These easing methods could be reproduced using cubic-bezier() values, [..] Not sure that it's worth the effort to support Drawer easing options via config?

On reflection, I'd like this to be as least breaking as possible. I have made a sub pr to reintroduce the easing as the suggested css cubic-bezier functions.

#602

It is not possible to represent the bounce and elastic easing functions using just cubic-bezier functions, so I have gracefully replaced those with their nearest cubic-bezier counter parts, back and quint respectively and have added warnings to the console to say as much.

References:
https://developer.mozilla.org/en-US/docs/Web/CSS/easing-function
https://easings.net/#
https://joshcollinsworth.com/blog/easing-curves

@oliverfoster oliverfoster marked this pull request as ready for review December 10, 2024 15:52
@oliverfoster
Copy link
Member Author

👍

@oliverfoster
Copy link
Member Author

@kirsty-hames

I will make sure that the default browser behaviour is utilised for the loop around. We should not stop the focus from looping around into the browser UI if that is expect.

I've fixed it so that we support the existing config.json:_accessibility._options_isPopupWrapFocusEnabled behaviour. It now wraps when that property is true (default).

kirsty-hames
kirsty-hames approved these changes Dec 17, 2024
View reviewed changes
Copy link
Contributor

@kirsty-hames kirsty-hames left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works great thanks 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@swashbuck swashbuck swashbuck left review comments

@lemmyadams lemmyadams lemmyadams approved these changes

@kirsty-hames kirsty-hames kirsty-hames approved these changes

Assignees

@oliverfoster oliverfoster

Labels
enhancement New feature or request
Projects
adapt_framework: The TODO Board
Status: Needs Reviewing
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use css to animate popups and the dialog element for accessibility
4 participants
@oliverfoster @swashbuck @kirsty-hames @lemmyadams